Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix metadata parsing in FigureExtractor. Closes #1764. #1765

Merged
merged 5 commits into from
Jul 14, 2020

Conversation

umesh-timalsina
Copy link
Contributor

No description provided.

1. Minor refactor of ordering of functions
Copy link
Contributor

@brollb brollb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple minor comments but nothing big.

There is one other minor thing that would probably be good but can be part of their own PR as it isn't pressing. I don't think we need to record this._metaNodesMap. The methods that use it could be rewritten to just use the client. For example, getMetaType could be rewritten to get the ID and then use getNode from the client. Checking if the node inherits from "Metadata" could also be checked by using a convenience method that walks up the inheritance tree and checks for a node that is both part of the meta and has the given name. This would likely be more performant than the current approach. Anyway, it isn't a huge deal so it doesn't need to be part of this PR.

src/common/viz/FigureExtractor.js Outdated Show resolved Hide resolved
src/common/viz/FigureExtractor.js Outdated Show resolved Hide resolved
1. Remove _metaNodesMap from FigureExtractor
2. Use .find instead of .filter followed by .pop in getMetadataChildrenIds
@umesh-timalsina
Copy link
Contributor Author

For example, getMetaType could be rewritten to get the ID and then use getNode from the client. Checking if the node inherits from "Metadata" could also be checked by using a convenience method that walks up the inheritance tree and checks for a node that is both part of the meta and has the given name. This would likely be more performant than the current approach.

This has been addressed.

@brollb brollb merged commit d383d00 into master Jul 14, 2020
@brollb brollb deleted the 1764-fig-extractor-metadata branch July 14, 2020 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants